Skip to content

Conversation

@sithmein
Copy link

@sithmein sithmein commented Dec 5, 2025

@geoand geoand requested a review from sberyozkin December 5, 2025 07:32
@quarkus-bot

This comment has been minimized.

@github-actions
Copy link

github-actions bot commented Dec 5, 2025

😭 Deploy PR Preview failed.

Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks very much @sithmein, LGTM, proposed minor updates only

Copy link
Member

@FroMage FroMage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to point out that there does not appear to be a consensus here, when I search online, and in fact many people point out that CORS is not a substitute for CSRF, and in particular CORS only applies to XHR and not to forms.

See https://stackoverflow.com/questions/19793695/does-a-proper-cors-setup-prevent-csrf-attack but that's not the only place I saw mentioning that.

So, at the very least, we should be much more nuanced in how we present the cases where CORS can help in some cases against some CSRF attacks.

@sithmein
Copy link
Author

sithmein commented Dec 5, 2025

The browser respecting CORS headers does indeed not apply to forms. But the server-side checks that are done by the CORS filter do prevent CSRF also for forms. Therefore it's "only" this part of the filter that is relevant for CSRF, setting the headers does not play a role.

@FroMage
Copy link
Member

FroMage commented Dec 5, 2025

But the server-side checks that are done by the CORS filter do prevent CSRF also for forms

How?

@sithmein
Copy link
Author

sithmein commented Dec 5, 2025

By checking whether Origin either matches Host or is part of a whitelist, similar to https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#using-standard-headers-to-verify-origin

@sithmein sithmein force-pushed the issue-51290-csrf-cors-docs branch 2 times, most recently from 6d4e92a to 69f88a8 Compare December 5, 2025 10:56
@FroMage
Copy link
Member

FroMage commented Dec 5, 2025

I guess it depends on whether CORS will reject requests when no Origin and Referer are present, is it the case?

@sithmein
Copy link
Author

sithmein commented Dec 5, 2025

CSRF is mostly only an issue in the browser because it automatically sends session cookies with every requests. And the browser always sets the Origin header. Requests without Origin usually come from other applications, such as curl (in the very simple case). There CSRF is not issue because there is no cookie in the first place.

@quarkus-bot

This comment has been minimized.

@sberyozkin
Copy link
Member

The browser respecting CORS headers does indeed not apply to forms. But the server-side checks that are done by the CORS filter do prevent CSRF also for forms.

@sithmein If form submissions are done from scripts, sure CORS filter will handle it, but HTML forms that are just part of simple HTML pages won't have Origin set if the page's origin is different, right ? So indeed as @FroMage recommends, we can highlight that the the proposed advice applies to XHR/Fetch only...

@sithmein
Copy link
Author

sithmein commented Dec 5, 2025

It's hard to find definite information but as far as I know, the Origin header is also set for cross-origin form requests. However, in such cases it's set to null instead of the real origin. But this doesn't interfere with the filter because null will hardly ever be an allowed origin.
See also https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Origin

@sberyozkin
Copy link
Member

@sithmein

It's hard to find definite information but as far as I know, the Origin header is also set for cross-origin form requests. However, in such cases it's set to null instead of the real origin. But this doesn't interfere with the filter because null will hardly ever be an allowed origin.

Sure.

Let me suggest a couple of other minor updates to stress the users have to be certain that Origin is indeed set in all cross-origin cases

@sberyozkin
Copy link
Member

@sithmein Have a look at the proposed updates please, I suppose we indeed should offer a rather strong recommendation for users to check it, to make sure users do not incidentally deploy the CORS filter where no Origin is available for whatever reasons

@sithmein sithmein force-pushed the issue-51290-csrf-cors-docs branch from f1279a7 to a68d147 Compare December 6, 2025 16:20
@quarkus-bot
Copy link

quarkus-bot bot commented Dec 6, 2025

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit a68d147.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add cross-references between CSRF and CORS filter

4 participants